Skip to content

fix(router): make wrapScreen public for RouterView compatibility (Closes #1928)#1940

Merged
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
madhuri-perumalla:fix/router-view-api
Jul 5, 2026
Merged

fix(router): make wrapScreen public for RouterView compatibility (Closes #1928)#1940
Karanjot786 merged 1 commit into
Karanjot786:mainfrom
madhuri-perumalla:fix/router-view-api

Conversation

@madhuri-perumalla

@madhuri-perumalla madhuri-perumalla commented Jul 2, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Description

This PR resolves the remaining API mismatch between Router and RouterView by exposing wrapScreen() as a public Router API.

Previously, RouterView relied on the private _wrapScreen() method, which caused TypeScript build failures due to access of a private member. This change introduces a public wrapScreen() method and updates internal references to use the public API.

Related Issue

Closes #1928

Which package(s)?

  • @termuijs/router

Type of Change

  • 🐛 Bug fix (type:bug)

Checklist

  • ⭐ You starred the repo.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if applicable).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

  • Yes, I am a GSSoC 2026 contributor.

Notes for the Reviewer

The autoUnmount property and direction field on NavigateEvent are already present in the current main branch. This PR focuses only on exposing wrapScreen() as a public API and updating all internal usages accordingly, providing a minimal, backward-compatible fix for the remaining API mismatch reported in Issue #1928.

Summary by CodeRabbit

  • Bug Fixes
    • Improved router screen rendering consistency across route changes, including back/forward navigation and not-found handling.
    • Updated route view behavior to use the shared screen-wrapping flow, helping keep navigation transitions consistent.

@github-actions github-actions Bot added the type:bug +10 pts. Bug fix. label Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f4412235-1ba6-4774-827c-2d3beb106929

📥 Commits

Reviewing files that changed from the base of the PR and between b307e74 and 09831b9.

📒 Files selected for processing (2)
  • packages/router/src/RouterView.tsx
  • packages/router/src/router.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/router/src/router.ts
  • packages/router/src/RouterView.tsx
📝 Walkthrough

Walkthrough

Router._wrapScreen is renamed to public wrapScreen, and all router navigation paths plus RouterView now call the renamed method.

Changes

wrapScreen API rename

Layer / File(s) Summary
Rename Router method and update call sites
packages/router/src/router.ts		 _wrapScreen is renamed to wrapScreen, and the not-found, matched-route, back, and forward flows now call the new method name.
Update RouterView consumer
packages/router/src/RouterView.tsx		 State initialization now calls router.wrapScreen(router.current) instead of the private _wrapScreen.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • Karanjot786/TermUI#1848: Introduced RouterView's dependency on the _wrapScreen helper, which this PR renames and exposes publicly.

Suggested labels: quality:clean, level:intermediate, area:router

Suggested reviewers: Karanjot786

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific, concise, and accurately summarizes the main change to expose wrapScreen for RouterView.
Description check ✅ Passed The PR description includes the required sections, linked issue, package, change type, checklist, and reviewer notes.
Linked Issues check ✅ Passed The code addresses the reported RouterView/Router API mismatch by exposing wrapScreen and updating internal call sites.
Out of Scope Changes check ✅ Passed The changes stay focused on RouterView/Router API compatibility and do not introduce unrelated code.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@madhuri-perumalla madhuri-perumalla changed the title fix(router): make wrapScreen public for RouterView compatibility (Closes ##1928) fix(router): make wrapScreen public for RouterView compatibility (Closes #1928) Jul 2, 2026
@Karanjot786

Karanjot786 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thanks — the wrapScreen public-API change itself is clean (RouteMatch and VNode are already public exports, no as any leak).

Issue:

  1. bench CI is failing
    Check: bench (run 28602287058) — the log shows a Node crash dump then "Performance benchmark regressed by ≥20% on at least one metric."
    Problem: something in this change (or its base) trips the perf gate, so CI isn't green.
    Fix: check whether the regression is real (re-run bench locally with bun bench / the repo's bench script); if it's a flake, push a rebase to re-trigger; if real, address the hot-path cost.

Checklist before re-requesting review:

  • bench green
  • Tests pass locally (run: bun vitest run)

@madhuri-perumalla

madhuri-perumalla commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@Karanjot786
Thanks for the review! I've rebased the branch with the latest upstream/main and force-pushed to re-trigger all CI checks. This change only exposes wrapScreen as a public API and updates internal references, so it shouldn't affect benchmark performance.

Checklist :

  • bench green
  • Tests pass locally (run: bun vitest run)

@Karanjot786 Karanjot786 merged commit b49dd14 into Karanjot786:main Jul 5, 2026
8 checks passed
@Karanjot786 Karanjot786 added gssoc:approved Approved PR. Earns +50 base points. quality:clean x 1.2 multiplier. Clean implementation. level:beginner +20 pts. Entry-level task. labels Jul 7, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Jul 7, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Karanjot786 Karanjot786 Awaiting requested review from Karanjot786 Karanjot786 is a code owner

Assignees

No one assigned

Labels

gssoc:approved Approved PR. Earns +50 base points. level:beginner +20 pts. Entry-level task. quality:clean x 1.2 multiplier. Clean implementation. type:bug +10 pts. Bug fix.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[GSSoC] RouterView component uses non-existent Router API methods

2 participants

@madhuri-perumalla @Karanjot786